-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
[v20.x] deps: V8: backport build fixes for Xcode 16.3 #58342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Bo98
wants to merge
3
commits into
nodejs:v20.x-staging
Choose a base branch
from
Bo98:v20.x-xcode16.3
base: v20.x-staging
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original commit message: [zlib][build] Remove fdopen #defines in zutil.h. The latest version of Clang changed what macros it predefines on Apple targets, causing errors about predefined macros in zlib. See: madler/zlib@4bd9a71 Bug: 1519899 Change-Id: Ie75ef4078f2c86d89ba6c036ddd13e768a40ccbb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5237020 Reviewed-by: Adenilson Cavalcanti <[email protected]> Commit-Queue: Hans Wennborg <[email protected]> Cr-Commit-Position: refs/heads/main@{#1253252} NOKEYCHECK=True GitOrigin-RevId: 2f39ac8d0a414dd65c0e1d5aae38c8f97aa06ae9 Refs: https://chromium.googlesource.com/chromium/src/third_party/zlib/+/646b7f569718921d7d4b5b8e22572ff6c76f2596
Original commit message: Define UChar as char16_t We used to have UChar defined as uint16_t which does not go along with STL these days if you try to have an std::basic_string<> of it, as there are no standard std::char_traits<> specialization for uint16_t. This switches UChar to char16_t where practical, introducing a few compatibility shims to keep CL size small, as (1) this would likely have to be back-ported and (2) crdtp extensively uses uint16_t for wide chars. Bug: b:296390693 Change-Id: I66a32d8f0050915225b187de56896c26dd76163d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4789966 Reviewed-by: Jaroslav Sevcik <[email protected]> Commit-Queue: Jaroslav Sevcik <[email protected]> Auto-Submit: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#89559} Refs: v8/v8@182d9c0
Original commit message: Fix build issue, remove unneeded include uchar.h. Follow the conversation on: https://groups.google.com/g/v8-dev/c/nsbshwlmP3c. The `uchar.h` include is not necessary. It was added to get the definition of char16_t but that's an intrinsic type in C++. Change-Id: I0aaa11dba0be3ccad15b9e421f8bae71450d443b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4823404 Reviewed-by: Omer Katz <[email protected]> Commit-Queue: Eric Leese <[email protected]> Reviewed-by: Eric Leese <[email protected]> Cr-Commit-Position: refs/heads/main@{#89787} Refs: v8/v8@1a3ecc2 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Review requested:
|
marco-ippolito
approved these changes
May 15, 2025
mertcanaltin
approved these changes
May 15, 2025
I confirm this PR fixes the compilation on my machine:
Currently the CI for v20.x is broken so errors are unrelated |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Node 20.19.2 currently fails to build from source since Xcode 16.3 (and LLVM Clang 18). This pull request backports three v8 commits that fix the issue:
fdopen
is a definition ifTARGET_OS_MAC
is defined, butfdopen
has never been a definition in the macOS SDK. The latest Clang compiler now definesTARGET_OS_MAC
globally, triggering this code and causing a compile error.git-node
doesn't do backports ofthird_party
repos so I did this one manuallystd::basic_string<uint16_t>
is invalid C++ without a customstd::char_traits
. This is now enforced in the latest libc++.All of the above commits are already included in Node 22 and later, hence the 20.x-only PR.